Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor: extract methods for parsing method input+output definitions #84

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

svandriel
Copy link
Contributor

As a first step to lowering the complexity of the parser, I extracted two methods for parsing the method input and output definitions.

Both functions(createMethodInputDefinition and createMethodOutputDefinition) take an options hash and produce the respective definitions.

Note that both functions cannot be reduced to one because their behavior is slightly different.

@dderevjanik
Copy link
Owner

Thank you for starting refactoring code. I wanted to make it simple as possible - having one function responsible for parsing input and other for generating output... but yeah, 180 commits later, so why not to start refactoring it little bit :) .

I have to find some time to do code review, but I like your approach 👍

@svandriel
Copy link
Contributor Author

svandriel commented May 13, 2024

Thank you for starting refactoring code. I wanted to make it simple as possible - having one function responsible for parsing input and other for generating output... but yeah, 180 commits later, so why not to start refactoring it little bit :) .

I have to find some time to do code review, but I like your approach 👍

I understand making it as simple as possible 👍

My reason was that reducing the (cognitive) complexity of the parser would also make it more simple.

I was thinking to move the 2 extracted functions createMethodInputDefinition and createMethodOutputDefinition to separate files as well (maybe in a parser directory)?

This would shorten the parser code as well, making it easier to read. In addition, the split off functions would also be easier to grasp in isolation.

What do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants